Skip to content

[Refactor] #537 - 솝탬프 UI 변경 #541

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 25 commits into from
Apr 10, 2025
Merged

Conversation

dlwogus0128
Copy link
Contributor

@dlwogus0128 dlwogus0128 commented Apr 7, 2025

🌴 PR 요약

솝탬프 UI를 다크모드로 변경하고, 피그마와 다른 일부 레이아웃 등을 수정했습니다.

🌱 작업한 브랜치

🌱 PR Point

✅ 솝탬프 다크모드로 변경

  • 전체적으로 솝탬프 기능을 피그마에 적용된 다크모드 색상으로 변경했습니다.
  • 솝탬프에 공통적으로 적용되어 있는 스타일들이 있는데 (e.g., 포인트 컬러, 비활성화 컬러 등) 이 색상들이 공통으로 관리되는 것이 아닌, 각각의 파일에서 적용되고 있어서 일일이 수정해주어야 했습니다.
  • 때문에 제가 놓친 컬러들이 있다면 리뷰 부탁드려요!

✅ 솝탬프 레이아웃 및 리소스 수정

  • 기존에도 피그마의 레이아웃과 달랐던 곳이 몇 군데 있더라구요...? 피그마와 최대한 같도록 레이아웃 수정했습니다.
  • 이미지 화질이 깨지거나, 컬러가 변경된 이미지들을 교체해 넣었습니다.

✅ 솝탬프 버그 수정

  • 노운 이슈였는데, 기존에 미션 날짜만 설정해도 등록 버튼이 활성화되는 문제가 있었습니다. (이 경우, 미션이 제대로 등록되지 않습니다.)
  • 코드 확인해보니 미션 날짜값이 변동될 때, (1) 미션 날짜값이 있는지 (2) 미션 내용 텍스트 값이 있는지 두 가지를 체크하더라구요.
  • 그런데, (2)에서 기본 Placeholder 값을 텍스트가 존재한다고 인식하고 있어 생긴 이슈였습니다.
  • 따라서 아래의 TO-BE처럼 현재 텍스트 값을 Placeholder와 비교하는 방식으로 수정했습니다.

+) 궁금한 점이 있습니다!
제가 지금 TO-BE로 지정한 것처럼 Placeholder 값과 비교하면, 사용자가 Placeholder 기본값과 같은 값을 입력할 경우 버튼이 비활성화 된 상태로 유지되는 문제가 있습니다. 요거 어떤 식으로 더 현명하게 해결할 수 있을까요?

.sink(receiveValue: { owner, date in
    owner.bottomButton.setEnabled(!date.isEmpty && owner.textView.hasText) // AS-IS
    owner.bottomButton.setEnabled(!date.isEmpty && !(owner.textView.text == I18N.ListDetail.memoPlaceHolder)) // TO-BE
})

✅ 기타

  • 끼워넣기로 솝트로그 앱기능 셀에 기본 이미지가 hot icon으로 되어 있었는데, 네트워크 연결이 느릴 경우 해당 부분이 빨간 색이라서 이미지가 깜박거리는 것처럼 어색하게 보이더라구요. 그래서 기본 이미지 삭제해뒀습니다!

📌 참고 사항

  • 솝탬프 코드가.. 상당해요.
  • spaceholder 값 한꺼번에 수정하는 방법 있나요? 코드 수정할 때마다 spaceholder이 다른 게 은근 귀찮네요.
  • 다른 코드들이랑 스타일이 달라서 수정하려면 대공사가 필요할 것 같은데... 우선은 필요한 부분들만 조금씩 수정했으니 그 부분 감안해서 리뷰해주시면 감사하겠습니다.
  • 저도 그동안 피그마에 지정되어 있는 컬러들을 컴포넌트에 그대로 지정하고 있었는데, 이렇게 다크모드처럼 전체 컬러 테마를 변경해야 할 때 유연하게 대응할 수 없다는 점을 여실하게 느꼈습니다. 포인트 / 비활성화 컬러 등처럼 어떤 테마에서 공통적으로 사용되는 컬러들에 대해서는 별도의 이름으로 정의해서, 공통으로 관리하는 것이 필요하다는 것을 갠적으로 느꼈습니다. 담부터는 이런 효율성까지도 생각하며 기능 개발을 해야겠어요~! 숑숑

글고 다크모드. 예쁘다. 헤헤 😍

📸 스크린샷

기능 스크린샷
솝탬프 UI 변경
Simulator.Screen.Recording.-.iPhone.16.Pro.Max.-.2025-04-07.at.17.20.45.mp4

📮 관련 이슈

@dlwogus0128 dlwogus0128 self-assigned this Apr 7, 2025
Copy link

height bot commented Apr 7, 2025

Link Height tasks by mentioning a task ID in the pull request title or commit messages, or description and comments with the keyword link (e.g. "Link T-123").

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

Copy link
Contributor

@meltsplit meltsplit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수고 많으셨습니다 :)
다크모드 대응하게 된다면 화이트가 화이트가 아닐수도 있겠군요... 허허...
근데 별도의 이름을 정하면 해당 namespace를 관리해야하는 번거로움이 있을 것 같기도 해서 고민이네요..

제가 지금 TO-BE로 지정한 것처럼 Placeholder 값과 비교하면, 사용자가 Placeholder 기본값과 같은 값을 입력할 경우 버튼이 비활성화 된 상태로 유지되는 문제가 있습니다. 요거 어떤 식으로 더 현명하게 해결할 수 있을까요?

저는 두가지 방법 떠오르네요!

  1. button의 enabled 조건을 isEdited && !text.isEmpty로 설정하는 방법
    여기서 isEdited는 수정시마다 초기값은 false고 수정시마다 true로 바꿔줄 프로퍼티입니다.
    너무 사소한 변수이기에 VC에 두고 사용해도 좋을 것 같아욥. 깔끔한 코드같진 않네요 ㅜㅠ

  2. textView를 viewModel의 input으로 넣고, Output으로 buttonEnabled를 반환하는 것도 좋아보이네욥.
    placeHolder를 VC에서 textView의 초기값으로만 둔다면, viewModel에서 isEdited와 같은 조건 없이 textViewText.isEmpty 로만 buttonEnabled로 이어지게 할 수 있지 않을까 라는 추측해봅니당


전 후자 방법인 viewmodel의 Output에 버튼의 활성화 여부 두는 것에 한표 던집니다.
이유는 버튼 활성화 여부는 나름 중요한 뷰로직인 것 같기 때문입니다~

Copy link
Contributor

@yungu0010 yungu0010 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

솝탬프 테마 바꾸는 작업이 번거로웠을텐데 레이아웃까지 확인해주셔서 감사합니다 !! 💕

의견 1

제가 지금 TO-BE로 지정한 것처럼 Placeholder 값과 비교하면, 사용자가 Placeholder 기본값과 같은 값을 입력할 경우 버튼이 비활성화 된 상태로 유지되는 문제가 있습니다. 요거 어떤 식으로 더 현명하게 해결할 수 있을까요?

저도 석우님 의견처럼 뷰 모델을 활용한 방법에 한 표 던집니다! 그러면 추가적인 필수 입력 값이 생기더라도 더 관리가 쉬워질 것 같아요.
textView 값과 date 값을 VM에 전달, 버튼 enabled 여부를 다시 VC에 알려주는 방식의 수도코드 적어보았는데 같이 참고해주세요 !

/// ViewModel 수도 코드
@Published date: String = ""
@Published textViewContents: String = ""

$date.combineLatest($contents)
    .sink(receiveValue: { date, contents in
        let isEnable = 버튼 활성화 로직
        output.isEnable.send(isEnable)
    }).store(in: cancelBag)

의견 2

spaceholder 값 한꺼번에 수정하는 방법 있나요? 코드 수정할 때마다 spaceholder이 다른 게 은근 귀찮네요.

spaceholder가 혹시 코드 indent를 말씀하시는걸까요? 맞다면 control + i단축키를 사용하면 재현님이 설정하신 간격으로 한 번에 수정할 수 있습니다.

의견 3

다른 코드들이랑 스타일이 달라서 수정하려면 대공사가 필요할 것 같은데... 우선은 필요한 부분들만 조금씩 수정했으니 그 부분 감안해서 리뷰해주시면 감사하겠습니다.

콕찌르기도 최근 Feature들과 코드 스타일이 많이 다른 상황이에요. 두 Feature는 컨벤션이나 플로우를 한 번 정리해볼 필요가 있을 것 같아요, 차차 예쁘게 만들어보아요 !!👍

저도 그동안 피그마에 지정되어 있는 컬러들을 컴포넌트에 그대로 지정하고 있었는데, 이렇게 다크모드처럼 전체 컬러 테마를 변경해야 할 때 유연하게 대응할 수 없다는 점을 여실하게 느꼈습니다. 포인트 / 비활성화 컬러 등처럼 어떤 테마에서 공통적으로 사용되는 컬러들에 대해서는 별도의 이름으로 정의해서, 공통으로 관리하는 것이 필요하다는 것을 갠적으로 느꼈습니다. 담부터는 이런 효율성까지도 생각하며 기능 개발을 해야겠어요~! 숑숑

재현님 말대로 STChartRectangleView에서처럼 상태에 따라 달라지는 값들은 enum으로 관리하면 유지보수할 때 편하겠네요 ! 추가로 soptampPurple100 같이 더 이상 사용하지 않는 색상은 삭제하는 거 어떠신가요?

@yungu0010
Copy link
Contributor

yungu0010 commented Apr 7, 2025

솝탬프 가이드 VC의 뒤로가기 기능이 없는 것 같아요. 뒤로가기와 스와이프 기능도 추가해주시면 감사하겠습니다!

솝탬프 가이드 pop
@dlwogus0128
Copy link
Contributor Author

dlwogus0128 commented Apr 8, 2025

@meltsplit @yungu0010 꼼꼼한 리뷰 감사드립니다 🙇🏼

버튼 활성화 로직에 관하여

남겨주신 리뷰들 읽어보면서 ListDetailVC 쪽 코드를 다시 확인해보았는데, 값들의 state에 따라 발생시켜줘야 하는 이벤트들에 관한 로직(e.g., 버튼 활성화 로직 등)이 전부 VC에 있더라구요. VC는 Input으로 VM에게 관찰 값이 변할 때마다 해당 값만 전달하고, VM은 이 값들을 받아 Output으로 버튼 활성화 여부를 알려주거나, 데이터를 post, put하는 requestModel을 만들도록 하는 등 로직들을 최대한 VM으로 옮겼습니다.

특히 버튼 활성화 로직의 경우, 아래와 같이 인풋값으로 들어오는 image, date, text를 CurrentValueSubject로 흘려보내서 셋 중 하나의 값이라도 변경될 때마다 감지하도록 설계했는데 이것보다 더 좋은 방법이 있을까요? Input에서 들어오는 값들을 그냥 묶으려고 했더니, CombineLatest는 세 스트림 모두 최소 한 번씩 값이 방출되어야 이후의 연산이 작동되기 때문에, edit State인 경우에는 버튼이 비활성화로 고정되는 문제가 있었습니다. 콤바인 천재 선배림들,, 알려주세요 ..

private let currentImage = CurrentValueSubject<Data, Never>(Data())
private let currentDate = CurrentValueSubject<String, Never>(String())
private let currentText = CurrentValueSubject<String, Never>(String())

// 버튼 활성화 로직
Publishers.CombineLatest3(currentImage, currentDate, currentText)
    .withUnretained(self)
    .filter { owner, _ in
        owner.sceneType != .completed
    }
    .map { owner, currentState in
        let (currentImage, currentDate, currentText) = currentState
        
        switch owner.sceneType {
        case .edit:
            let isImageSelected = owner.isImageSelected(currentImage)
            let isDateChanged = owner.isDateChanged(output.listDetailModel?.activityDate, currentDate)
            let isTextChanged = owner.isTextChanged(output.listDetailModel?.content, currentText)
            return isImageSelected || isDateChanged || isTextChanged
        default:
            let isImageSelected = !currentImage.isEmpty
            let isDateChanged = !currentDate.isEmpty
            let isTextChanged = !currentText.isEmpty
            return isImageSelected && isDateChanged && isTextChanged
        }
    }
    .sink { isEdited in
        output.bottomButtonEnabled.send(isEdited)
    }
    .store(in: cancelBag)

코드 indent

감사합니다 .. StampFeature에 관련 코드들 전부 수정해놨어요!

사용하지 않는 DS 색상 삭제

이것도 솝탬프만 DS가 따로 되어 있어서.. 삭제하는 과정이 필요할 것 같습니다!

스와이프

솝탬프 가이드 VC의 뒤로가기 기능이 없는 것 같아요. 뒤로가기와 스와이프 기능도 추가해주시면 감사하겠습니다!

이거 기획의도인 줄 알고 추가 안해뒀는데... 그냥 누락이었나봐요?! ㅋㅋ 수정해뒀습니다.~

@dlwogus0128 dlwogus0128 merged commit eaca8fc into develop Apr 10, 2025
@dlwogus0128 dlwogus0128 deleted the refactor/#537-soptamp-ui branch April 10, 2025 05:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Refactor] 솝탬프 UI 변경
3 participants